Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: sector indexing and data retrieval #688

Merged
merged 40 commits into from
Feb 17, 2025

Conversation

cernicc
Copy link
Member

@cernicc cernicc commented Jan 21, 2025

Description

Add sector indexing on the successful prove commitment and integrates retrieval server that uses the indexing when retrieving blocks.

The #675 should be solved before this is merged. The reason is that the client doesn't know currently which blocks it should request after the root node.

Checklist

  • Make sure that you described what this change does.
  • Have you tested this solution?
  • Were there any alternative implementations considered?
  • Did you document new (or modified) APIs?

@cernicc cernicc linked an issue Jan 21, 2025 that may be closed by this pull request
@cernicc cernicc force-pushed the feat/635/provider-stroage-index branch 3 times, most recently from 800b896 to cdb22b3 Compare January 23, 2025 14:44
@cernicc cernicc force-pushed the feat/635/provider-stroage-index branch from 379156b to cc5a9bc Compare January 28, 2025 16:51
@cernicc cernicc changed the title feat: integrate retrieval server feat: sector indexing and data retrieval Feb 3, 2025
@cernicc cernicc marked this pull request as ready for review February 3, 2025 11:17
@cernicc cernicc self-assigned this Feb 3, 2025
@cernicc cernicc added the ready for review Review is needed label Feb 3, 2025
Copy link
Contributor

@th7nder th7nder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huuge piece of work!
I left some major questions in the comments and have another one.
How is this thing tested?

@cernicc cernicc added the ready for review Review is needed label Feb 17, 2025
Copy link
Collaborator

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I think there are (IMO) some small refactors to be made that make the whole PR smaller and more understandable

@cernicc cernicc requested a review from jmg-duarte February 17, 2025 15:17
@jmg-duarte jmg-duarte added ready for review Review is needed and removed ready for review Review is needed labels Feb 17, 2025
Copy link
Collaborator

@jmg-duarte jmg-duarte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final touchups

@cernicc cernicc added ready for review Review is needed and removed ready for review Review is needed labels Feb 17, 2025
@cernicc cernicc requested a review from jmg-duarte February 17, 2025 15:52
aidan46
aidan46 previously approved these changes Feb 17, 2025
jmg-duarte
jmg-duarte previously approved these changes Feb 17, 2025
@jmg-duarte jmg-duarte enabled auto-merge (squash) February 17, 2025 16:01
@cernicc cernicc dismissed stale reviews from jmg-duarte and aidan46 via 751acbd February 17, 2025 16:25
@aidan46 aidan46 self-requested a review February 17, 2025 16:32
@cernicc cernicc added ready for review Review is needed and removed ready for review Review is needed labels Feb 17, 2025
@jmg-duarte jmg-duarte merged commit f62cd11 into develop Feb 17, 2025
6 checks passed
@jmg-duarte jmg-duarte deleted the feat/635/provider-stroage-index branch February 17, 2025 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review Review is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: retrieval server blockstore Index sectors and pieces on the storage provider
5 participants